-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document contract execution semantics #898
Conversation
ethanfrey
commented
Apr 22, 2021
•
edited
Loading
edited
- SDK/Tendermint Environment
- Basic Execution
- Handling Messages
- Handling Submessages
- Query Semantics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent writeup 👍 👍 👍
SEMANTICS.md
Outdated
area in Ethereum. | ||
|
||
The `reply` call may return `Err` itself, in which case it is treated like the | ||
caller errored, and aborting the transaction. (Unless, of course, the original |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Unless, of course, the original...
The sentence in brackets make it hard for me to follow the flow. Let's get the reply flow described first and explain this at the end of the paragraph?
Maybe with an example:
Please keep in mind that submessage
execution
and reply
can happen within the context of another submessage
. For example contract-A --submessage--> contract-B --submessage->contract-C
.
Then contract-B
can revert the state for contract-C
and itself by returning an error in the submessage reply
handling. contract-A
can revert state for itself, contract-B
and contract-C
with it's submessage reply
error result..
SEMANTICS.md
Outdated
just as if the original `execute` and returned `data: Some(b"better idea")`. If | ||
`reply` returns `data: None`, it will not modify any previously set data state. | ||
If there are multiple submessages all setting this, only the last one is used | ||
(they all overwrite any previous `data` value). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some details on multiple submessages would be good:
A contract may return multiple submessages
with different ReplyOn
settings. For example
a) submessage with reply on success
b) submessage with reply on failure
If we return a,b then we have the following conditions:
processing a) | processing b) | reply called | may overwrite result from reply |
---|---|---|---|
ok | ok | a) | a) |
err | err | b) | b |
err | ok | none | none |
ok | err | a)b) | b) |
Any error returned when handling the replies
in the contract, would abort the process and revert the state as if the original execute
failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost. The first submessage to non handle an error, or return an error in reply will be treated like an error via message, thus aborting the transaction.
processing a) | processing b) | reply called | may overwrite result from reply | note |
---|---|---|---|---|
ok | ok | a) | a) | returns success |
err | err | none | none | returns error (abort parent transaction) |
err | ok | none | none | returns error (abort parent transaction) |
ok | err | a)b) | a)b) | if both a) and b) overwrite, only b) will be used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. This makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replied
SEMANTICS.md
Outdated
just as if the original `execute` and returned `data: Some(b"better idea")`. If | ||
`reply` returns `data: None`, it will not modify any previously set data state. | ||
If there are multiple submessages all setting this, only the last one is used | ||
(they all overwrite any previous `data` value). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost. The first submessage to non handle an error, or return an error in reply will be treated like an error via message, thus aborting the transaction.
processing a) | processing b) | reply called | may overwrite result from reply | note |
---|---|---|---|---|
ok | ok | a) | a) | returns success |
err | err | none | none | returns error (abort parent transaction) |
err | ok | none | none | returns error (abort parent transaction) |
ok | err | a)b) | a)b) | if both a) and b) overwrite, only b) will be used |
SEMANTICS.md
Outdated
In the Cosmos SDK, a transaction returns a number of events to the user, along | ||
with an optional data "result". This result is hashed into the next block hash | ||
to be provable and can return some essential state (although in general client | ||
apps rely on Events more). This result is more commonly used to pass results | ||
between contracts or modules in the sdk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are events hashed into the app state as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No they are not. There was a long discussion about that a few months ago.
Just Data
and Code
(0 = success) are hashed. They are stored in ResultHash (not AppHash, which is only the state the app writes, not other info that goes to tendermint). I can explain this a bit more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, thanks. I don't think we need to go into details here but worth making that fact explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! This is very helpful for educating developers!
Are there any code changed expected for the reply
change? I guess it all happens in wasmd.
It's all wasmd and @alpe is on that. We needed to define what was expected behavior first. I will address the comments and prepare to merge |
d4059d6
to
59b6e8c
Compare
Okay, this is complete as far as I am concerned. In integrated your feedback and added a small section about queries. Please review and suggest any last changes to approve it. I will rebase the IBC doc on this and refer to these new techniques. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool
Nice document. Thanks for writing it. |